Skip to content

Return INDETERMINATE when signer issuer is not in trusted list#13

Merged
edgarsj merged 3 commits into
mainfrom
fix/trustlist-validity-response
Mar 17, 2026
Merged

Return INDETERMINATE when signer issuer is not in trusted list#13
edgarsj merged 3 commits into
mainfrom
fix/trustlist-validity-response

Conversation

@edgarsj

@edgarsj edgarsj commented Mar 17, 2026

Copy link
Copy Markdown
Owner

Summary

  • When a trustListProvider is configured and the signer's issuer is not found (or not trusted at signing time), downgrade overall status from VALID to INDETERMINATE with an ISSUER_NOT_TRUSTED limitation
  • Previously, trustlist results were not factored into isValid/status, so a document signed with a test/demo certificate would show as VALID even when trustlist checking was enabled
  • Add test-environment-sig-sample.edoc as a committed fixture (demo certificate, not in trusted list)
  • Tests cover both public and sensitive invalid sample directories, verifying overall status, trustListMatch, limitations, and full checklist item statuses

Test plan

  • 124 unit/integration tests pass
  • New tests verify INDETERMINATE status + ISSUER_NOT_TRUSTED limitation when trustlist enabled
  • New tests verify VALID status without trustlist provider (crypto-only)
  • New tests verify all checklist items for untrusted issuer scenario
  • Existing valid sample tests still pass with trustlist enabled

When a trustListProvider is configured and the signer's issuer is not
found (or not trusted at signing time), downgrade overall status from
VALID to INDETERMINATE with an ISSUER_NOT_TRUSTED limitation.

Add test-environment-sig-sample.edoc fixture with tests for both
public and sensitive sample dirs.
@greptile-apps

greptile-apps Bot commented Mar 17, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes a gap where trust-list results were not factored into the final status/isValid fields of verifySignature. Documents signed with certificates whose issuer is absent from (or not active in) the configured trust list would previously report VALID; they now correctly report INDETERMINATE with an ISSUER_NOT_TRUSTED limitation. A similar downgrade is added for trust-list provider errors (TRUST_LIST_CHECK_FAILED). A committed demo-certificate fixture enables the new integration tests to run without sensitive samples.

Key changes:

  • isValid promoted from const to let in verifySignature so the post-status-determination block can mutate it.
  • New downgrade block (lines 1533–1555) triggers only when status === "VALID" and a trustListProvider is configured; handles three cases: issuer not found, found but not trusted at signing time, and provider error.
  • tests/integration/batch_edoc.test.ts gains a reusable verifyUntrustedIssuer helper that covers both the public invalid_samples fixture and an optional sensitive directory.
  • The trustedAtTime === false strict-equality guard in the downgrade condition can silently pass a TrustListMatch where found: true but trustedAtTime is undefined (e.g. from a custom provider); the checklist would show indeterminate for that item while the overall status remains VALID.

Confidence Score: 4/5

  • Safe to merge with a minor fix recommended for the trustedAtTime === undefined edge case in the downgrade condition.
  • The core logic is correct for all cases exercised by the tests and the built-in trust-list matcher (which always sets trustedAtTime when query.time is supplied). The one identified gap — a TrustListMatch where found: true but trustedAtTime is omitted — is not reachable via the built-in provider in normal operation but is reachable through a custom TrustListProvider, creating a mismatch between status: "VALID" and an indeterminate checklist item. The fix is a one-line change to the guard condition.
  • The downgrade block in src/core/verification.ts around line 1536 deserves attention regarding the trustedAtTime === false strict equality check.

Important Files Changed

Filename Overview
src/core/verification.ts Adds trust-list downgrade logic after the existing status-determination block; handles both the "not found" and "error" cases correctly, but a strict === false check on trustedAtTime can leave status as VALID while the checklist item shows "indeterminate" for custom providers that omit trustedAtTime.
tests/integration/batch_edoc.test.ts Adds a reusable verifyUntrustedIssuer helper and two describe blocks (public and sensitive) that correctly validate INDETERMINATE status, ISSUER_NOT_TRUSTED limitation, and full checklist item statuses for test-cert documents; test logic is solid and the skipping guard is correct.
tests/fixtures/invalid_samples/test-environment-sig-sample.edoc New binary fixture signed with a demo/test certificate not present in the trusted list; serves as the committed sample for the INDETERMINATE-status integration tests.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[verifySignature called] --> B[Crypto validation\ncertResult + checksumResult\n+ signatureResult + timestamp]
    B --> C{isValid?}
    C -- No --> D[status = INVALID /\nINDETERMINATE /\nUNSUPPORTED\nbased on failure reason]
    C -- Yes --> E[status = VALID]
    E --> F{trustListProvider\nconfigured?}
    F -- No --> G[Return VALID]
    F -- Yes --> H{trustListMatch\navailable?}
    H -- Yes --> I{found AND\ntrustedAtTime\n≠ false?}
    I -- found=false OR\ntrustedAtTime=false --> J[status = INDETERMINATE\nlimitations += ISSUER_NOT_TRUSTED]
    I -- found=true AND\ntrustedAtTime=true --> G
    I -- found=true AND\ntrustedAtTime=undefined --> K[⚠️ status stays VALID\nbut checklist =\nindeterminate]
    H -- No → trustListError set --> L[status = INDETERMINATE\nlimitations += TRUST_LIST_CHECK_FAILED]
    H -- No, no error --> G
    J --> M[Return INDETERMINATE]
    L --> M
    D --> N[Return non-VALID]
Loading

Last reviewed commit: aa40d1a

Comment thread src/core/verification.ts Outdated
Comment thread tests/integration/batch_edoc.test.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7b0877a6c9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/core/verification.ts Outdated
edgarsj added 2 commits March 17, 2026 21:04
Increase test timeout for untrusted issuer tests (trust list loading
is slow on CI). Also downgrade to INDETERMINATE when the trust list
provider throws an error, rather than silently returning VALID.
Use trustedAtTime === false instead of !trustedAtTime to avoid
false-downgrading providers that return { found: true } without the
trustedAtTime field. Remove stray console.log from test.
@edgarsj

edgarsj commented Mar 17, 2026

Copy link
Copy Markdown
Owner Author

@greptileai @codex please recheck fixes

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 👍

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@edgarsj edgarsj merged commit 89d60d5 into main Mar 17, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant